Updated PHP, Swoole, Imagemagick, Add more CI scanning, ...#63
Updated PHP, Swoole, Imagemagick, Add more CI scanning, ...#63davidjeddy merged 23 commits intomainfrom
Conversation
…DM.md, and test.yaml. This rebuild will pull imagemagick > 7.1 to address CVEs as found by Trivy.
📝 WalkthroughWalkthroughThis pull request substantially restructures the CI/CD infrastructure and Docker build process. It introduces multi-architecture builds with per-branch manifest handling, enhances vulnerability scanning with Trivy SARIF reports, adds container validation workflows, extends PHP extensions in the base image, and establishes security and documentation policies. Changes
Sequence Diagram(s)sequenceDiagram
actor Developer
participant GitHub as GitHub Actions
participant BuildArch as Builder<br/>(per-arch)
participant Registry as Docker Registry
participant Manifest as Manifest<br/>Service
Developer->>GitHub: Push to main/feature
alt Main Branch Push
GitHub->>BuildArch: Build image (amd64)
BuildArch->>Registry: Push image:sha-amd64
GitHub->>BuildArch: Build image (arm64)
BuildArch->>Registry: Push image:sha-arm64
GitHub->>Manifest: Create manifest from both
Manifest->>Registry: Push manifest:release-tag
else Feature Branch Push
GitHub->>BuildArch: Build image (amd64)
BuildArch->>Registry: Push image:sha-amd64
GitHub->>BuildArch: Build image (arm64)
BuildArch->>Registry: Push image:sha-arm64
GitHub->>Manifest: Create manifest from both
Manifest->>Registry: Push manifest:commit-sha
end
GitHub->>GitHub: Parallel: Run Trivy, Dive, Structure Tests
GitHub->>Registry: Scan for vulnerabilities
GitHub->>Registry: Analyze image efficiency
GitHub->>Registry: Validate container structure
sequenceDiagram
participant Workflow as Trivy Workflow
participant Docker as Docker
participant Trivy as Trivy Scanner
participant CodeQL as GitHub CodeQL
participant Storage as SARIF Storage
Workflow->>Docker: Build image
Docker-->>Workflow: Image ready
Workflow->>Trivy: Scan image for vulnerabilities
Trivy->>Trivy: Generate SARIF report
Trivy-->>Workflow: trivy-image-results.sarif
Workflow->>CodeQL: Upload SARIF results
CodeQL->>Storage: Store security findings
CodeQL-->>Workflow: Results published
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
d15cdbf to
955d067
Compare
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
4efc25a to
e7c0e31
Compare
|
✅ Created PR with unit tests: #64 |
ad41bf1 to
6ac7505
Compare
6ac7505 to
fa5df86
Compare
… they fail. But main branch should always be successful.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr-scan.yml:
- Around line 3-24: The workflow currently uses pull_request_target which causes
the Build job to checkout the base branch (actions/checkout@v4) and perform a
Docker build with elevated permissions (see job "build" and permissions like
security-events: write); change the trigger for the scanning/build path from
pull_request_target to pull_request (or split the workflow so privileged
publish/SARIF upload remains under pull_request_target while the untrusted
Docker build runs under pull_request) and ensure you do not checkout the PR head
with write-scoped permissions in the pull_request_target job.
In `@CHANGES.md`:
- Line 5: The changelog entry currently references the wrong filename
"CHANGELOG.md"; update the text in CHANGES.md to point to the correct file name
"CHANGES.md" (replace the string "CHANGELOG.md" with "CHANGES.md") so links and
references resolve to the existing changelog file.
In `@Dockerfile`:
- Line 1: The Dockerfile hardcodes the PHP ABI suffix
"no-debug-non-zts-20240924" in the COPY operations (the entries that reference
that string) which will mismatch the declared BASEIMAGE
"php:8.5.3-cli-alpine3.23"; change the Dockerfile to compute the extension
directory dynamically (e.g., run php -r 'echo ini_get("extension_dir");' or
similar at build-time and store it in a shell variable/ARG/ENV like EXT_DIR) and
use that variable in the COPY targets instead of the literal
"no-debug-non-zts-20240924", updating the COPY lines that reference the
hardcoded suffix so they write into the actual extension directory returned by
PHP.
In `@README.md`:
- Line 74: Fix the typo in the README note by changing the sentence "Note: Build
of the image and push to the registry shoudl be handle by automation." to use
correct wording: "Note: Build of the image and push to the registry should be
handled by automation." Ensure only the misspelled words "shoudl" -> "should"
and "handle" -> "handled" are corrected in that sentence.
- Around line 39-41: Update the README examples that pipe commands into tee so
the pipeline exit status reflects the first command (e.g., docker build, trivy,
container-structure-test, docker push): add set -o pipefail at the start of each
example and redirect stderr to stdout for timed commands (e.g., time docker
build ... 2>&1 | tee ...) so failures are not masked by tee. Also correct the
repeated typo "shoudl be handle" to "should be handled" in the specified
occurrences.
In `@tests.yaml`:
- Line 7: The test is asserting an exact ImageMagick patch version in tests.yaml
(expectedOutput: [".*ImageMagick 7.1.2.15"]) which will break when Alpine
updates packages; either loosen the matcher or pin the package in Dockerfile:
update tests.yaml's expectedOutput to a more tolerant regex (e.g.,
".*ImageMagick 7\\.1\\.2\\." or ".*ImageMagick 7\\.1\\.") to allow patch
updates, or modify the Dockerfile entries that install imagemagick (both the
build and final stage apk add/install and any apk upgrade invocations) to pin
the package to the exact version you want (use the Alpine package release
suffix) so the installed ImageMagick stays deterministic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c961301c-1344-4213-8bac-e71afd856b68
📒 Files selected for processing (8)
.dockerignore.github/workflows/pr-scan.yml.github/workflows/trivy.yml.gitignoreCHANGES.mdDockerfileREADME.mdtests.yaml
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/trivy.yml (1)
37-42:⚠️ Potential issue | 🟠 MajorUse
format: sarifinstead of template syntax.The
format: 'template'withtemplate: '@/contrib/sarif.tpl'is not compatible with trivy-action 0.35.0. The correct SARIF configuration is:format: sarif output: 'trivy-results.sarif'Remove the
templatefield—it is not a valid input for this version.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/trivy.yml around lines 37 - 42, Update the Trivy action step that uses aquasecurity/trivy-action@0.35.0: remove the invalid "template" input and set "format" to sarif (i.e., replace format: 'template' + template: '@/contrib/sarif.tpl' with format: sarif) while keeping output: 'trivy-results.sarif'; locate the step by the uses: aquasecurity/trivy-action@0.35.0 entry to make the change.
🧹 Nitpick comments (2)
TODO.md (1)
12-12: Minor: Remove leading space in code span.The code span has a space before the pipe character which triggers MD038.
Suggested fix
-- capture build logs via ` | tee "build-$(date +%s).log"` +- capture build logs via `| tee "build-$(date +%s).log"`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TODO.md` at line 12, Remove the leading space inside the inline code span containing the pipe command so the code reads `| tee "build-$(date +%s).log"` (no space before the |); locate the inline snippet shown as ` | tee "build-$(date +%s).log"` in the TODO.md content and edit it to eliminate the extra space within the backticks to satisfy MD038.README.md (1)
3-3: Travis CI badge may be outdated.The repository uses GitHub Actions workflows (test.yml, trivy.yml, build-and-push.yml, pr-scan.yml), not Travis CI. Consider updating to a GitHub Actions badge.
Suggested replacement
-[](https://travis-ci.com/appwrite/docker-base) +[](https://github.com/appwrite/docker-base/actions/workflows/test.yml)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 3, Replace the outdated Travis CI badge markdown reference "[](https://travis-ci.com/appwrite/docker-base)" in README.md with a GitHub Actions badge that points to the appropriate workflow(s) (e.g., test.yml or build-and-push.yml); remove the travis URL and add the corresponding actions badge(s) using the GitHub Actions badge URL pattern for the workflow names so the README reflects the current CI system.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr-scan.yml:
- Around line 1-3: The workflow titled "PR Security Scan" currently triggers on
all pushes (the `on: push` stanza) which mismatches the name and may duplicate
scans from `trivy.yml`; update this workflow by either changing its trigger from
`push` to `pull_request` (if it should run only on PRs), removing the workflow
entirely if `trivy.yml` already covers the same scans, or renaming the workflow
title to reflect branch push scanning; look for the workflow name "PR Security
Scan", the `on: push` trigger, and the separate `trivy.yml` workflow to make the
appropriate change.
In `@CHANGES.md`:
- Line 9: Fix the typo in CHANGES.md by updating the line that reads "base_image
and php_build_date to containber labels" to use the correct word "container" so
it reads "base_image and php_build_date to container labels"; modify the exact
string in CHANGES.md to correct "containber" → "container".
In `@Dockerfile`:
- Around line 194-195: The Dockerfile uses $TZ in the RUN ln -snf
/usr/share/zoneinfo/$TZ ... command but TZ is never declared; add a build-time
ARG (e.g., ARG TZ="UTC") and/or ENV TZ="${TZ}" before that RUN step so $TZ is
defined (or remove the two lines if timezone is configured elsewhere); update
the Dockerfile around the RUN ln -snf invocation to reference the declared
ARG/ENV TZ to ensure the symlink and /etc/timezone write succeed.
- Around line 22-59: The RUN layer in the Dockerfile reuses the ineffective
pattern "apk add --no-cache --virtual .deps" followed by a separate "&& apk add
--no-cache" which prevents packages from being installed into the ".deps"
virtual package; change the RUN sequence so the packages intended as build
dependencies are passed directly to the same "apk add --no-cache --virtual
.deps" invocation (i.e., move the list of build tools/libraries such as
autoconf, automake, g++, gcc, make, openssl-dev, postgresql-dev, etc. into the
.deps invocation) and only keep a separate non-virtual apk add for runtime
packages if needed, ensuring the virtual name ".deps" actually contains the
build-time packages to be removable later.
- Around line 197-235: The .deps virtual package is created empty and later
deleted, so fix the RUN block in the Dockerfile: either remove the "apk add
--no-cache --virtual .deps" and the trailing "&& apk del .deps" if all listed
packages are needed at runtime, or group true build-only packages (e.g., g++,
gcc, make, autoconf, automake, libstdc++, pkg-config-like build deps) into the
virtual package and keep runtime packages outside it so that "apk del .deps"
actually removes only build-only dependencies; update the RUN command
accordingly to reflect the chosen option.
In `@README.md`:
- Around line 39-41: Update the README entry that currently shows the incorrect
Docker command string "docker-buildx build --no-cache --tag appwrite/base:latest
." to the correct invocation "docker buildx build --no-cache --tag
appwrite/base:latest ."; locate the shell block in README.md (the code fence
containing the time docker-buildx line) and replace the hyphenated command with
the space-separated "docker buildx" form so the example uses the proper Docker
Buildx CLI.
---
Outside diff comments:
In @.github/workflows/trivy.yml:
- Around line 37-42: Update the Trivy action step that uses
aquasecurity/trivy-action@0.35.0: remove the invalid "template" input and set
"format" to sarif (i.e., replace format: 'template' + template:
'@/contrib/sarif.tpl' with format: sarif) while keeping output:
'trivy-results.sarif'; locate the step by the uses:
aquasecurity/trivy-action@0.35.0 entry to make the change.
---
Nitpick comments:
In `@README.md`:
- Line 3: Replace the outdated Travis CI badge markdown reference "[](https://travis-ci.com/appwrite/docker-base)"
in README.md with a GitHub Actions badge that points to the appropriate
workflow(s) (e.g., test.yml or build-and-push.yml); remove the travis URL and
add the corresponding actions badge(s) using the GitHub Actions badge URL
pattern for the workflow names so the README reflects the current CI system.
In `@TODO.md`:
- Line 12: Remove the leading space inside the inline code span containing the
pipe command so the code reads `| tee "build-$(date +%s).log"` (no space before
the |); locate the inline snippet shown as ` | tee "build-$(date +%s).log"` in
the TODO.md content and edit it to eliminate the extra space within the
backticks to satisfy MD038.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 931d0f90-4158-4617-a98c-5038190e049b
📒 Files selected for processing (8)
.github/workflows/build-and-push.yml.github/workflows/pr-scan.yml.github/workflows/test.yml.github/workflows/trivy.ymlCHANGES.mdDockerfileREADME.mdTODO.md
8f5bf13 to
2ebf44f
Compare
8fe4f52 to
0f7f450
Compare
ede58b2 to
1bff3bd
Compare
1bff3bd to
9ff192d
Compare
Greptile SummaryThis PR upgrades the Appwrite base Docker image to PHP 8.5.3 (via the
Confidence Score: 1/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant GH as GitHub Push Event
participant BAP as build_and_push<br/>(if: ref != main)
participant MFF as manifest_build_and_push_on_feature<br/>(if: ref != main)
participant MFM as manifest_build_and_push_on_main<br/>(if: ref == main)
participant DH as DockerHub
Note over GH,DH: Feature branch push (ref != main)
GH->>BAP: trigger (condition met)
BAP->>BAP: Build amd64 + arm64 images
BAP->>DH: Push sha-amd64 + sha-arm64
BAP->>MFF: needs satisfied → trigger
MFF->>DH: Create + push sha manifest
Note over GH,DH: Main branch push (ref == main) — BROKEN
GH->>BAP: trigger (condition FALSE → skipped)
BAP--xMFM: needs: build_and_push skipped → MFM also skipped
Note over MFM: manifest_build_and_push_on_main<br/>never runs on main
Note over MFM: github.event.release.tag_name<br/>also empty in push context
|
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (5)
README.md (2)
82-84:⚠️ Potential issue | 🟡 MinorPipeline exit code may be masked by
tee.Without
set -o pipefail, the shell returnstee's exit status rather thandocker push's. If the push fails, the pipeline may still report success.Suggested fix
```bash +set -o pipefail docker push appwrite/base:latest | tee "push-$(date +%s).log" ```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 82 - 84, The pipeline may mask docker push failures because the command uses a pipe to tee; add shell strictness so failures propagate by inserting set -o pipefail (or set -euo pipefail) before the pipeline line that runs docker push appwrite/base:latest | tee "push-$(date +%s).log" so the shell returns docker push's exit code instead of tee's; ensure this change is added in README examples or any script snippets that run this pipeline.
80-80:⚠️ Potential issue | 🟡 MinorFix grammar: "should be handle" → "should be handled".
Suggested fix
-Pushing a built image to a repository should be handle by automation. +Pushing a built image to a repository should be handled by automation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 80, Fix the grammar in the README by changing the phrase "Pushing a built image to a repository should be handle by automation." to "Pushing a built image to a repository should be handled by automation." — update the sentence text in the README (search for the exact phrase "should be handle") and replace it with the corrected "should be handled"..github/workflows/lifecycle-policy.yml (1)
18-36:⚠️ Potential issue | 🟡 MinorWorkflow has no functional cleanup action.
A previous reviewer noted this workflow doesn't do anything currently. The entire cleanup logic (lines 28-36) is commented out, leaving only a DockerHub login step. Either implement the cleanup action or remove this workflow until it's ready.
Also, line 3 has a typo: "ordred" should be "ordered".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/lifecycle-policy.yml around lines 18 - 36, The workflow job prune_sha_tagged_images currently only logs into DockerHub and the actual cleanup steps (the actions/delete-package-versions step and associated ignore-versions/min-versions-to-keep/package settings) are commented out, and there is also a typo in the workflow metadata ("ordred" → "ordered"); either implement the cleanup by uncommenting and validating the actions/delete-package-versions step and its inputs (ignore-versions, min-versions-to-keep, package-name, package-type) and adding any required pre-pull/backup steps, or remove the prune_sha_tagged_images job entirely until ready; also fix the typo by replacing "ordred" with "ordered" wherever it appears in the workflow.Dockerfile (2)
185-223:⚠️ Potential issue | 🟠 MajorFinal-stage
.depscleanup is ineffective and leaves build tools in runtime image.The pattern at Line 188 (
--virtual .depswith no packages) followed by a separateapk addmeans.depsis empty; Line 222 removes nothing. This keeps compiler/build packages in the final image.Suggested fix
RUN \ apk update && \ apk upgrade && \ - apk add --no-cache --virtual .deps \ - && apk add --no-cache \ + apk add --no-cache --virtual .build-deps \ autoconf \ automake \ + g++ \ + gcc \ + make \ + linux-headers \ + openssl-dev \ + postgresql-dev \ + curl-dev \ + zstd-dev \ + yaml-dev \ + lz4-dev \ + imagemagick-dev \ + libjpeg-turbo-dev \ + libpng-dev \ + libjxl-dev \ + libmaxminddb-dev \ + jpeg-dev \ + && apk add --no-cache \ brotli-dev \ certbot \ - curl-dev \ docker-cli \ docker-cli-compose \ - g++ \ - gcc \ git \ imagemagick \ imagemagick-heic \ - jpeg-dev \ libavif \ libgomp \ libheif \ - libjpeg-turbo-dev \ - libjxl-dev \ - libmaxminddb-dev \ - libpng-dev \ libstdc++ \ libwebp \ - linux-headers \ - lz4-dev \ - make \ - openssl-dev \ - postgresql-dev \ rsync \ - yaml-dev \ zip \ - zstd-dev \ - && apk del .deps \ + && apk del .build-deps \ && rm -rf /var/cache/apk/*🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 185 - 223, The RUN block defines a virtual package `.deps` with no packages then installs all packages in a separate apk add, so `apk del .deps` is a no-op and build tools remain in the final image; fix by grouping build-only packages into a named virtual package (e.g., `.build-deps`) when installing (include compilers and dev libs like g++, gcc, make, autoconf, automake, *-dev, etc.) and install runtime packages separately, then run `apk del .build-deps` before cleanup; update the RUN block that contains `--virtual .deps` and the subsequent `apk add` so the virtual name actually wraps the build-only packages or use a two-step install where `.build-deps` is removed prior to `rm -rf /var/cache/apk/*`.
182-183:⚠️ Potential issue | 🟠 Major
TZis used before declaration.Line 182 and Line 183 reference
$TZ, but noARG TZ/ENV TZis declared in this file, so timezone setup is unreliable.Suggested fix
+ARG TZ="UTC" +ENV TZ="${TZ}" + RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && \ echo $TZ > /etc/timezone🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 182 - 183, The RUN uses $TZ before it's declared; add a build ARG and/or environment variable for TZ and set a sensible default so the ln/echo steps use a defined value—e.g., declare ARG TZ (with a default like UTC) and then set ENV TZ=$TZ (or at minimum ENV TZ=UTC) before the RUN that references $TZ so the timezone setup in RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && echo $TZ > /etc/timezone reliably uses a defined TZ.
🧹 Nitpick comments (3)
.github/workflows/trivy.yml (2)
4-4: Trivy scan triggers on all branches.
on: pushwithout branch filters will run scans on every push to any branch, which may consume significant CI resources. Consider restricting to main/release branches if full-branch scanning isn't required:on: push: branches: [ "main" ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/trivy.yml at line 4, The workflow currently triggers Trivy scans on every push via the top-level "on: push" event; restrict this to specific branches by replacing the bare "on: push" trigger with a push event that includes a branches filter (e.g., only "main" or release branches) so scans run only for those branches and avoid running on all branch pushes.
37-38: Samesarif_file: '.'concern asscheduled-trivy.yml.Consider using the explicit filename
trivy-image-results.sariffor clarity and predictability.Suggested fix
- name: Upload Trivy scan results uses: github/codeql-action/upload-sarif@v4 with: - sarif_file: '.' + sarif_file: 'trivy-image-results.sarif'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/trivy.yml around lines 37 - 38, Replace the ambiguous sarif_file value (currently set to '.') with an explicit filename to make outputs predictable; update the key sarif_file to "trivy-image-results.sarif" in the workflow step (the with: block that contains sarif_file) so the Trivy action writes a named SARIF file, and do the same change in any sibling workflow (scheduled-trivy.yml) if present to keep behavior consistent..github/workflows/scheduled-trivy.yml (1)
43-44: Use explicit SARIF filename for clarity.The Trivy step outputs
trivy-image-results.sarif, so usingsarif_file: 'trivy-image-results.sarif'instead of'.'makes the intention explicit and improves maintainability by matching the declared output filename.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/scheduled-trivy.yml around lines 43 - 44, The workflow currently sets the Trivy action output with sarif_file: '.' which is ambiguous; update the Trivy step to explicitly reference the generated SARIF filename (e.g., set sarif_file: 'trivy-image-results.sarif') so the step consuming the artifact matches the actual output name and improves clarity in the scheduled-trivy.yml workflow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-and-push.yml:
- Around line 16-17: The workflow prevents the manifest job from running on main
and uses a release-only field in a push event; update the job dependency and
event usage: ensure the job manifest_build_and_push_on_main does not depend only
on build_and_push when running on refs/heads/main (e.g., remove or conditionally
gate the needs: build_and_push for main or add a separate job path that runs on
main), and change the workflow trigger or payload references so
github.event.release.tag_name is only used when the workflow is triggered by
release events (either switch to on: release or guard access to
github.event.release.tag_name with a conditional that reads from the push ref
when on: push). Adjust the conditional at the top (the github.ref check) and the
manifest steps to use the correct tag variable for each event to make the
manifest creation reachable and avoid undefined tag values.
In @.github/workflows/dive.yml:
- Around line 23-26: The Dive step uses yuichielectric/dive-action@0.0.4 but
doesn't pass the repository's .dive-ci policy file, so thresholds won't be
enforced; update the Dive job step (the step named "Dive" that currently sets
the with.image input) to include the action input that specifies the config file
and point it to ".dive-ci" (e.g., add a with entry like config: ".dive-ci" or
config-file: ".dive-ci" depending on the action's input name) so the action
loads the repository policy when running against the image.
In `@CHANGES.md`:
- Around line 18-19: Replace all occurrences of the product name spelled
"Github" with the correct, consistent "GitHub" in the changelog entries;
specifically update the two lines shown in the diff (“.github/*.yml steps
updated to latest versions” and any other entries noted at lines 24-25 and 39)
so the product name uses the correct capitalization ("GitHub") everywhere in
CHANGES.md.
In `@README.md`:
- Line 3: Replace the Travis CI badge markdown in the README with a GitHub
Actions badge that references the main workflow name build-and-push.yml: locate
the existing badge line containing "travis" or the exact string "[](https://travis-ci.com/appwrite/docker-base)"
and update it to use the GitHub Actions badge URL for the repository and
workflow (pointing to the build-and-push.yml workflow) so the README shows the
correct Actions build status and link.
---
Duplicate comments:
In @.github/workflows/lifecycle-policy.yml:
- Around line 18-36: The workflow job prune_sha_tagged_images currently only
logs into DockerHub and the actual cleanup steps (the
actions/delete-package-versions step and associated
ignore-versions/min-versions-to-keep/package settings) are commented out, and
there is also a typo in the workflow metadata ("ordred" → "ordered"); either
implement the cleanup by uncommenting and validating the
actions/delete-package-versions step and its inputs (ignore-versions,
min-versions-to-keep, package-name, package-type) and adding any required
pre-pull/backup steps, or remove the prune_sha_tagged_images job entirely until
ready; also fix the typo by replacing "ordred" with "ordered" wherever it
appears in the workflow.
In `@Dockerfile`:
- Around line 185-223: The RUN block defines a virtual package `.deps` with no
packages then installs all packages in a separate apk add, so `apk del .deps` is
a no-op and build tools remain in the final image; fix by grouping build-only
packages into a named virtual package (e.g., `.build-deps`) when installing
(include compilers and dev libs like g++, gcc, make, autoconf, automake, *-dev,
etc.) and install runtime packages separately, then run `apk del .build-deps`
before cleanup; update the RUN block that contains `--virtual .deps` and the
subsequent `apk add` so the virtual name actually wraps the build-only packages
or use a two-step install where `.build-deps` is removed prior to `rm -rf
/var/cache/apk/*`.
- Around line 182-183: The RUN uses $TZ before it's declared; add a build ARG
and/or environment variable for TZ and set a sensible default so the ln/echo
steps use a defined value—e.g., declare ARG TZ (with a default like UTC) and
then set ENV TZ=$TZ (or at minimum ENV TZ=UTC) before the RUN that references
$TZ so the timezone setup in RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime
&& echo $TZ > /etc/timezone reliably uses a defined TZ.
In `@README.md`:
- Around line 82-84: The pipeline may mask docker push failures because the
command uses a pipe to tee; add shell strictness so failures propagate by
inserting set -o pipefail (or set -euo pipefail) before the pipeline line that
runs docker push appwrite/base:latest | tee "push-$(date +%s).log" so the shell
returns docker push's exit code instead of tee's; ensure this change is added in
README examples or any script snippets that run this pipeline.
- Line 80: Fix the grammar in the README by changing the phrase "Pushing a built
image to a repository should be handle by automation." to "Pushing a built image
to a repository should be handled by automation." — update the sentence text in
the README (search for the exact phrase "should be handle") and replace it with
the corrected "should be handled".
---
Nitpick comments:
In @.github/workflows/scheduled-trivy.yml:
- Around line 43-44: The workflow currently sets the Trivy action output with
sarif_file: '.' which is ambiguous; update the Trivy step to explicitly
reference the generated SARIF filename (e.g., set sarif_file:
'trivy-image-results.sarif') so the step consuming the artifact matches the
actual output name and improves clarity in the scheduled-trivy.yml workflow.
In @.github/workflows/trivy.yml:
- Line 4: The workflow currently triggers Trivy scans on every push via the
top-level "on: push" event; restrict this to specific branches by replacing the
bare "on: push" trigger with a push event that includes a branches filter (e.g.,
only "main" or release branches) so scans run only for those branches and avoid
running on all branch pushes.
- Around line 37-38: Replace the ambiguous sarif_file value (currently set to
'.') with an explicit filename to make outputs predictable; update the key
sarif_file to "trivy-image-results.sarif" in the workflow step (the with: block
that contains sarif_file) so the Trivy action writes a named SARIF file, and do
the same change in any sibling workflow (scheduled-trivy.yml) if present to keep
behavior consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e204670c-3eae-4161-8bbb-d99fe0904e2d
📒 Files selected for processing (14)
.dive-ci.github/workflows/build-and-push.yml.github/workflows/dive.yml.github/workflows/lifecycle-policy.yml.github/workflows/scheduled-trivy.yml.github/workflows/structure-test.yml.github/workflows/test.yml.github/workflows/trivy.ymlCHANGES.mdDockerfileREADME.mdSECURITY.mdTODO.mdtests.yaml
💤 Files with no reviewable changes (1)
- .github/workflows/test.yml
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
CHANGES.md (2)
23-24:⚠️ Potential issue | 🟡 MinorUse consistent product naming:
GitHub(capital H).The official product name uses a capital "H". Also at line 38.
,
📝 Suggested fix
-* Github action for container-structure-test now uses a marketplace action -* Github action runners pinned to Ubuntu 24.04 +* GitHub Action for container-structure-test now uses a marketplace action +* GitHub Action runners pinned to Ubuntu 24.04🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGES.md` around lines 23 - 24, Replace the inconsistent product name "Github" with the correct "GitHub" in the CHANGES.md entries shown (e.g., update the strings "Github action for container-structure-test now uses a marketplace action" and "Github action runners pinned to Ubuntu 24.04") and also fix the other occurrence noted around line 38 so all mentions use "GitHub".
38-38:⚠️ Potential issue | 🟡 MinorFix grammar and capitalization.
"Github" should be "GitHub", and "Setup" as a verb should be lowercase "set up".
📝 Suggested fix
-* Github action to Setup QEMU as GitHub now provides native ARM runners +* GitHub Action to set up QEMU as GitHub now provides native ARM runners🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGES.md` at line 38, Update the CHANGES.md entry to fix capitalization and grammar: change "Github" to "GitHub" and use the verb form "set up" instead of "Setup" so the line reads something like "GitHub action to set up QEMU as GitHub now provides native ARM runners." Ensure only the two words are corrected and preserve the rest of the sentence..github/workflows/build-and-push.yml (1)
67-87:⚠️ Potential issue | 🔴 Critical
manifest_build_and_push_on_mainusesgithub.event.release.tag_namebut will only run on push events.The job condition
if: github.ref == 'refs/heads/main'(line 68) matches push events to main, but lines 81 and 87 referencegithub.event.release.tag_name, which is undefined for push events. This will cause the manifest creation and push steps to fail with empty tag values.Additionally, for release events,
github.refis typically the tag ref (e.g.,refs/tags/v1.0.0), notrefs/heads/main, so this job won't run on releases either.,
🐛 Suggested fix: Use conditional tagging based on event type
manifest_build_and_push_on_main: - if: github.ref == 'refs/heads/main' + if: github.event_name == 'release' needs: build_and_push runs-on: ubuntu-24.04 steps:Alternatively, if you want to publish to main on every push, use
github.shainstead of the release tag:- name: Create manifest run: | docker manifest create \ - ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ github.event.release.tag_name }} \ + ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:latest \ ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ github.sha }}-amd64 \ ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ github.sha }}-arm64 - name: Push manifest run: | - docker manifest push ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ github.event.release.tag_name }} + docker manifest push ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:latest🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-and-push.yml around lines 67 - 87, The manifest_build_and_push_on_main job is using github.event.release.tag_name but is gated with if: github.ref == 'refs/heads/main' so release tag is undefined on push; change the job to compute a TAG variable based on the event (e.g., if github.event_name == 'release' use github.event.release.tag_name else use github.sha or github.ref name) and use that TAG in the docker manifest create and push commands, or alternatively broaden the job condition to run on both push and release events and branch the steps accordingly; update references to github.event.release.tag_name in the Create manifest and Push manifest steps to use the computed TAG (and keep github.sha-based fallbacks) so the manifest commands never receive an empty tag.
🧹 Nitpick comments (1)
.github/workflows/trivy.yml (1)
38-45: Consider using the explicit SARIF filename in the upload step.The Trivy step outputs to
trivy-image-results.sarif(line 38), but the upload step usessarif_file: '.'which uploads all SARIF files in the current directory. While this works, using the explicit filename would be more precise and avoid accidentally uploading unintended files.♻️ Suggested fix
- name: Upload Trivy scan results uses: github/codeql-action/upload-sarif@v4 with: - sarif_file: '.' + sarif_file: 'trivy-image-results.sarif'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/trivy.yml around lines 38 - 45, Update the "Upload Trivy scan results" step to reference the explicit SARIF filename instead of using the directory wildcard; change the sarif_file input for the upload step (the step named "Upload Trivy scan results" that uses github/codeql-action/upload-sarif@v4) to point to 'trivy-image-results.sarif' so only that specific file is uploaded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/trivy.yml:
- Line 27: The workflow references a non-existent runner version "uses:
actions/checkout@v6.0.2"; replace that tag with a valid one such as "uses:
actions/checkout@v5" to track the latest v5.x release or pin explicitly to
"uses: actions/checkout@v5.0.1" to fix the invalid version; update the uses
entry in the Trivy workflow where "actions/checkout@v6.0.2" appears.
---
Duplicate comments:
In @.github/workflows/build-and-push.yml:
- Around line 67-87: The manifest_build_and_push_on_main job is using
github.event.release.tag_name but is gated with if: github.ref ==
'refs/heads/main' so release tag is undefined on push; change the job to compute
a TAG variable based on the event (e.g., if github.event_name == 'release' use
github.event.release.tag_name else use github.sha or github.ref name) and use
that TAG in the docker manifest create and push commands, or alternatively
broaden the job condition to run on both push and release events and branch the
steps accordingly; update references to github.event.release.tag_name in the
Create manifest and Push manifest steps to use the computed TAG (and keep
github.sha-based fallbacks) so the manifest commands never receive an empty tag.
In `@CHANGES.md`:
- Around line 23-24: Replace the inconsistent product name "Github" with the
correct "GitHub" in the CHANGES.md entries shown (e.g., update the strings
"Github action for container-structure-test now uses a marketplace action" and
"Github action runners pinned to Ubuntu 24.04") and also fix the other
occurrence noted around line 38 so all mentions use "GitHub".
- Line 38: Update the CHANGES.md entry to fix capitalization and grammar: change
"Github" to "GitHub" and use the verb form "set up" instead of "Setup" so the
line reads something like "GitHub action to set up QEMU as GitHub now provides
native ARM runners." Ensure only the two words are corrected and preserve the
rest of the sentence.
---
Nitpick comments:
In @.github/workflows/trivy.yml:
- Around line 38-45: Update the "Upload Trivy scan results" step to reference
the explicit SARIF filename instead of using the directory wildcard; change the
sarif_file input for the upload step (the step named "Upload Trivy scan results"
that uses github/codeql-action/upload-sarif@v4) to point to
'trivy-image-results.sarif' so only that specific file is uploaded.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b08e1977-2324-4587-b5a3-bba05809c8cb
📒 Files selected for processing (3)
.github/workflows/build-and-push.yml.github/workflows/trivy.ymlCHANGES.md
82a41e6 to
60392ba
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
Dockerfile (2)
188-223:⚠️ Potential issue | 🟠 Major
apk del .depsis ineffective with the current install pattern.The
--virtual .depsinstall is empty, while actual packages are installed in a separate command, so Line 222 removes nothing from the final image.Suggested fix
RUN \ apk update && \ apk upgrade && \ - apk add --no-cache --virtual .deps \ - && apk add --no-cache \ + apk add --no-cache --virtual .deps \ autoconf \ automake \ brotli-dev \ certbot \ curl-dev \ docker-cli \ docker-cli-compose \ g++ \ gcc \ git \ imagemagick \ imagemagick-dev \ imagemagick-heic \ jpeg-dev \ libavif \ libgomp \ libheif \ libjpeg-turbo-dev \ libjxl-dev \ libmaxminddb-dev \ libpng-dev \ libstdc++ \ libwebp \ linux-headers \ lz4-dev \ make \ openssl-dev \ postgresql-dev \ rsync \ yaml-dev \ zip \ zstd-dev \ && apk del .deps \ && rm -rf /var/cache/apk/*🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 188 - 223, The .deps virtual package is never populated because --virtual .deps is applied to an empty apk add command, so apk del .deps removes nothing; update the Dockerfile so the large apk add that installs build packages uses --virtual .deps (i.e., move the --virtual .deps token to the apk add that lists autoconf automake ... zstd-dev) so that those packages are grouped under the .deps virtual package and can be removed with apk del .deps, or alternatively remove the apk del .deps line if you intend to keep those packages in the final image; refer to the apk add/ del commands and the .deps virtual name when making the change.
182-183:⚠️ Potential issue | 🟠 MajorDeclare
TZbefore timezone setup.
$TZis used on Line 182/183 without being declared in this stage, so timezone configuration can be incorrect.Suggested fix
+ARG TZ="UTC" +ENV TZ=${TZ} RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && \ echo $TZ > /etc/timezone🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 182 - 183, The Dockerfile uses the TZ variable in the RUN command that performs timezone setup (ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && echo $TZ > /etc/timezone) but TZ is not declared in this stage; declare TZ earlier (e.g., add an ARG TZ=UTC or ENV TZ=UTC before that RUN) or pass it via build ARG and then ENV (ARG TZ; ENV TZ=$TZ) so the RUN can expand $TZ reliably, providing a sensible default like UTC.CHANGES.md (1)
38-38:⚠️ Potential issue | 🟡 MinorReword the QEMU removal note for readability.
Line 38 is hard to parse. Suggested wording:
GitHub action to set up QEMU removed because GitHub now provides native ARM runners.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGES.md` at line 38, Replace the hard-to-read changelog entry "GitHub action to Setup QEMU as GitHub now provides native ARM runners" with the clearer wording "GitHub action to set up QEMU removed because GitHub now provides native ARM runners" (update the exact line in CHANGES.md that currently contains that sentence).README.md (1)
3-3:⚠️ Potential issue | 🟡 MinorReplace the stale Travis badge with a GitHub Actions badge.
Line 3 still points to Travis CI, which no longer reflects this repo’s CI status.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 3, Replace the stale Travis CI badge markdown in README.md (the existing "[](https://travis-ci.com/appwrite/docker-base)" snippet) with the GitHub Actions badge for this repository: update both the image URL and the link target to point to your repository's workflow badge and workflow page (e.g., use "https://github.com/<OWNER>/<REPO>/actions/workflows/<WORKFLOW>.yml/badge.svg" for the image and "https://github.com/<OWNER>/<REPO>/actions/workflows/<WORKFLOW>.yml" for the link), preserving the same "[](...)" markdown structure..github/workflows/build-and-push.yml (1)
45-87:⚠️ Potential issue | 🔴 CriticalFix manifest job gating for push vs release events.
Current conditions mix branch checks with release-only context fields. On push to
main,github.event.release.tag_nameis undefined; on release events, theon_mainjob won’t run because the ref is a tag, notrefs/heads/main.Suggested fix
manifest_build_and_push_on_feature: - if: github.ref != 'refs/heads/main' + if: github.event_name == 'push' && github.ref != 'refs/heads/main' needs: build_and_push runs-on: ubuntu-24.04 @@ manifest_build_and_push_on_main: - if: github.ref == 'refs/heads/main' + if: github.event_name == 'release' && github.event.action == 'published' needs: build_and_push runs-on: ubuntu-24.04🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-and-push.yml around lines 45 - 87, The manifest jobs mix branch-based gating and release-only context fields; update the two jobs so the feature job keeps its branch check (manifest_build_and_push_on_feature with if: github.ref != 'refs/heads/main') but never references release fields, while the release job uses the release event condition (change manifest_build_and_push_on_main to trigger when github.event_name == 'release' or if: github.event_name == 'release' && github.ref startsWith 'refs/tags/') and use github.event.release.tag_name for the manifest create/push there; for the main-branch push path keep a separate job with if: github.ref == 'refs/heads/main' that uses github.sha (or github.ref_name) for tags and does not reference github.event.release.tag_name so release-only fields are only accessed in the release-scoped job.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@TODO.md`:
- Line 5: Fix the typo in the TODO line that currently reads "cache it, the use
it downstream": change "the" to "then" so the line reads "cache it, then use it
downstream"; locate the TODO text containing "cache it, the use it downstream"
and update it accordingly.
---
Duplicate comments:
In @.github/workflows/build-and-push.yml:
- Around line 45-87: The manifest jobs mix branch-based gating and release-only
context fields; update the two jobs so the feature job keeps its branch check
(manifest_build_and_push_on_feature with if: github.ref != 'refs/heads/main')
but never references release fields, while the release job uses the release
event condition (change manifest_build_and_push_on_main to trigger when
github.event_name == 'release' or if: github.event_name == 'release' &&
github.ref startsWith 'refs/tags/') and use github.event.release.tag_name for
the manifest create/push there; for the main-branch push path keep a separate
job with if: github.ref == 'refs/heads/main' that uses github.sha (or
github.ref_name) for tags and does not reference github.event.release.tag_name
so release-only fields are only accessed in the release-scoped job.
In `@CHANGES.md`:
- Line 38: Replace the hard-to-read changelog entry "GitHub action to Setup QEMU
as GitHub now provides native ARM runners" with the clearer wording "GitHub
action to set up QEMU removed because GitHub now provides native ARM runners"
(update the exact line in CHANGES.md that currently contains that sentence).
In `@Dockerfile`:
- Around line 188-223: The .deps virtual package is never populated because
--virtual .deps is applied to an empty apk add command, so apk del .deps removes
nothing; update the Dockerfile so the large apk add that installs build packages
uses --virtual .deps (i.e., move the --virtual .deps token to the apk add that
lists autoconf automake ... zstd-dev) so that those packages are grouped under
the .deps virtual package and can be removed with apk del .deps, or
alternatively remove the apk del .deps line if you intend to keep those packages
in the final image; refer to the apk add/ del commands and the .deps virtual
name when making the change.
- Around line 182-183: The Dockerfile uses the TZ variable in the RUN command
that performs timezone setup (ln -snf /usr/share/zoneinfo/$TZ /etc/localtime &&
echo $TZ > /etc/timezone) but TZ is not declared in this stage; declare TZ
earlier (e.g., add an ARG TZ=UTC or ENV TZ=UTC before that RUN) or pass it via
build ARG and then ENV (ARG TZ; ENV TZ=$TZ) so the RUN can expand $TZ reliably,
providing a sensible default like UTC.
In `@README.md`:
- Line 3: Replace the stale Travis CI badge markdown in README.md (the existing
"[](https://travis-ci.com/appwrite/docker-base)"
snippet) with the GitHub Actions badge for this repository: update both the
image URL and the link target to point to your repository's workflow badge and
workflow page (e.g., use
"https://github.com/<OWNER>/<REPO>/actions/workflows/<WORKFLOW>.yml/badge.svg"
for the image and
"https://github.com/<OWNER>/<REPO>/actions/workflows/<WORKFLOW>.yml" for the
link), preserving the same "[](...)" markdown structure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 117abfc4-191e-47da-a8ae-56a57d9016b0
📒 Files selected for processing (7)
.dive-ci.yml.github/workflows/build-and-push.yml.github/workflows/dive.ymlCHANGES.mdDockerfileREADME.mdTODO.md
… README.md, and test.yaml. This rebuild will pull imagemagick > 7.1 to address CVEs as found by Trivy.
What does this PR do?
Test Plan
Used the new image to
docker-compose upthe appwrite/appwrite project and am able to load the web applications landing page.Related PRs and Issues
Have you read the Contributing Guidelines on issues?
yes
Multi-arch build results
Summary by CodeRabbit
New Features
Documentation
Tests
Chores